Simplify fluid api recognize digit #10308
Conversation
jetfuel
requested review from
cs2be,
abhinavarora,
helinwang,
daming-lu and
sidgoyal78
| BATCH_SIZE = 64 | ||
|
|
||
|
|
||
| def inference_network(): |
There was a problem hiding this comment.
Could you rename inference_network to inference_program and train_network to train_program? Yi pointed out that "program" is more suitable for Fluid than "network".
| acc_val = numpy.array(acc_set).mean() | ||
| avg_loss_val = numpy.array(avg_loss_set).mean() | ||
| if float(acc_val) > 0.2: # Smaller value to increase CI speed | ||
| trainer.params.save(save_dirname) |
There was a problem hiding this comment.
Can you change trainer.params.save to trainer.save_params? (https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/fluid/tests/book/word2vec/no_test_word2vec_new_api.py#L103)
|
|
||
| def event_handler(event): | ||
| if isinstance(event, fluid.EndIteration): | ||
| print(event.metrics) |
There was a problem hiding this comment.
Can you change print(event.metrics) to avg_cost, acc = event.values? (e.g., same name as the return value of train_network), and rename avg_loss_val and acc_val below to avg_cost and acc for clarity.
| params = fluid.Params(save_dirname) | ||
| place = fluid.CUDAPlace(0) if use_cuda else fluid.CPUPlace() | ||
|
|
||
| inferencer = fluid.Inferencer(inference_network, params, place=place) |
There was a problem hiding this comment.
Sorry that the API has changed slightly, please change Inferencer and Trainer's constructor according to https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/fluid/tests/book/word2vec/no_test_word2vec_new_api.py#L118
jetfuel
force-pushed
the
simplify_fluid_api_recognize_digit
branch
from
8f1c70b
to
e68cc6e
Compare
|
@helinwang Updated according to review. |
| avg_cost = numpy.array(avg_cost_set).mean() | ||
| if float(acc) > 0.2: # Smaller value to increase CI speed | ||
| trainer.save_params(save_dirname) | ||
| return |
There was a problem hiding this comment.
This return actually does not stop training, perhaps remove it.
Not related to this PR, maybe we need a function called trainer.stop_train.
| return | ||
| else: | ||
| print( | ||
| 'BatchID {1:04}, Test Loss {2:2.2}, Acc {3:2.2}'.format( |
There was a problem hiding this comment.
are 1:04, 2:2.2, 3:2.2 typos?
There was a problem hiding this comment.
Good catch! They are from the original code. When I removed the first item in the print, I forgot to change the others. Will fix this.
|
|
||
| else: | ||
| print( | ||
| 'BatchID {1:04}, Test Loss {2:2.2}, Acc {3:2.2}'.format( |
There was a problem hiding this comment.
are 1:04, 2:2.2, 3:2.2 typos?
jetfuel
force-pushed
the
simplify_fluid_api_recognize_digit
branch
from
646e65a
to
c06e1ef
Compare
Update several variable names to make them more consistant. Update the Inferencer construction call.
jetfuel
force-pushed
the
simplify_fluid_api_recognize_digit
branch
from
c06e1ef
to
c10d030
Compare
No description provided.